ci(npm): smoke supported Node release lines - #3988

sequenceDiagram
  participant RuntimeSupport as runtime-support.ts
  participant VersionJob as npm-smoke-node-versions
  participant SmokeJob as npm-install-smoke
  RuntimeSupport->>VersionJob: provide NPM_SMOKE_NODE_VERSIONS
  VersionJob->>SmokeJob: export JSON Node-version matrix
  SmokeJob->>SmokeJob: run npm install smoke for each Node version

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

📦 Client bundle boundary

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@coderabbitai review

@codex review

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 2519e1f1fe

Deep review — merge confidence 78/100

Verdict: code is right; needs a trivial deno.json re-merge (keep both new test:scripts entries) before the queue will take it. A couple of design-weight nits.

What I verified (isolated worktree at 2519e1f)

• The workflow's deno eval snippet run verbatim → node_versions=["22","24"], exit 0. CI run 32604109349: contract job 16s, legs pass 2m52s / 2m54s.
• scripts/build/runtime-support.test.ts → 1 passed. deno task test:scripts → 151 passed / 2 failed; both failures reproduce on the merge-base (npm-package-metadata.test.ts needs npm/ built; generate-api-reference alias drift is the unpinned-local-Deno artifact). Not this PR.
• deno fmt --check --config=scripts/test.deno.json, deno lint, deno check on the new files: clean.
• Gating is correct: cicd.yml:851-859 prerelease.needs and :989-997 release.needs both include tests-npm-install-smoke, and neither if: (:850, :988) uses always(), so a single failed leg (even with fail-fast: false, :527) skips both publish jobs. Output wiring :504-505 / :518 >> "$GITHUB_OUTPUT" / :529 fromJSON(...) is right; an empty output would make fromJSON error at job setup rather than silently running zero legs. Fork PRs: both jobs share the same if:, so skip propagates.
• test:scripts list: PR = merge-base (49) + runtime-support.test.ts; main = merge-base + run-suite.test.ts. Nothing dropped — the conflict resolution must keep both.

Mergeability

Conflicts with origin/main in deno.json — same test:scripts line. Keep both entries.

Non-blocking

1. Heavier than needed (judgement). The contract test already parses cicd.yml, so a static matrix: node-version: ["22", "24"] asserted equal to NPM_SMOKE_NODE_VERSIONS would satisfy Potential fix for 1 code quality finding #731's "one maintainable contract" without a serial extra job (checkout + setup-deno + eval, +16s wall, +1 runner), a shell-embedded TS snippet, and fromJSON indirection.
2. "One contract" is incomplete: seven other node-version: "24" literals remain (cicd.yml:98,134,154,181,568,878,1055, incl. tests-node). Within Potential fix for 1 code quality finding #731's non-goals, but runtime-support.ts:4's doc comment shouldn't imply the constant governs CI Node generally.
3. Doc comment runtime-support.ts:12-19 matches chore: add Socket.dev CI integration for automatic PR scanning #748's table; "only works from 22.14.0 up" slightly over-claims (22.13.x untested) — say "22.14.0 was the first version observed to pass". engines.node still ships >=22.3.0 (build-npm-dnt.ts:177); pre-existing, correctly deferred to chore: add Socket.dev CI integration for automatic PR scanning #748 rather than hidden, and the floor's release line is enforced by construction (NPM_SMOKE_NODE_VERSIONS[0] = MINIMUM_NODE_RELEASE_LINE, asserted at runtime-support.test.ts:106-109).
4. record()/steps() (runtime-support.test.ts:16-30) are a third copy of asRecord/asSteps (scripts/ci/setup-deno-workflow.test.ts:40-52) and yamlRecord (runtime-inference-critical-flow.test.ts:82). Shared helper eventually.
5. Cost: legs run in parallel, so wall-clock adds ~16–30s; runner-minutes ~+3 min/PR. typecheck:consumer and the artifact-size report (:545-549) now run per leg and the size table lands in the step summary twice — restrict to one leg.
6. Neither new file is in the hand-listed fmt:check task, so the body's "deno task fmt:check" is vacuous for them (they're clean anyway).

Spec

All five #731 acceptance criteria met literally: Node 22 + 24 lines; both legs run the unchanged npm-install-smoke.sh (:551); release/prerelease depend on the matrix; versions from one module. No creep.

Not re-verified

Local packed smoke on 22/24 (CI legs are the evidence); the 9-version #748 table.

Reviewed with Claude Code.

Deep review

Merge confidence: 40/100
Recommendation: REQUEST CHANGES
Reviewed head: 2519e1f1fe02 against current origin/main

Findings

• MEDIUM / BLOCK: the PR is currently conflicting in deno.json:562. This branch adds scripts/build/runtime-support.test.ts to test:scripts; current main adds scripts/test/run-suite.test.ts and suite-runner changes. Rebase/merge and preserve both entries, then rerun the script suite.
• MEDIUM / architecture watch: .github/workflows/cicd.yml:525 replaces the stable tests (npm install smoke) check name with matrix-specific Node 22/24 names. If branch protection or merge queue keys on the old context, that required check disappears. Prefer a small stable aggregate job with the old name.
• LOW: CURRENT_CI_NODE_RELEASE_LINE is currently canonical only for npm-smoke coverage; several other CI jobs still pin Node 24. Narrow the naming/comment or centralize those uses later.

Standards

Pass otherwise. Matrix values are static, actions are pinned, and no shell/Actions injection path was found.

Spec

Pass. The matrix resolves to Node 22 and 24, both run the unchanged packed install smoke, and prerelease/stable release depend on the aggregate matrix result.

Architecture

BLOCK only because the current head is not integrable with main; the dynamic matrix design itself is sound.

Verification

• Runtime support test/check/lint/format/diff checks: passed
• Matrix eval: ["22","24"]
• Both GitHub Node smoke legs passed
• GitHub reports CONFLICTING; local git merge-tree confirms the conflict

Addressed the review follow-up on 371636f4d:

• merged current main and kept both scripts/test/run-suite.test.ts and scripts/build/runtime-support.test.ts in test:scripts
• renamed the current-release constant so its name matches the npm-smoke boundary
• clarified that 22.14.0 is the first observed passing version and 22.13.x remains unverified
• renamed the focused test so it claims release-line coverage, not exact-floor coverage

The previous smoke display name is not required by the active branch protection or required-check ruleset. The exact-floor packaging defect remains tracked by issue #748.

Final-head verification passed: deno task test:scripts (155 tests, 509 steps), 12 tool-search tests, deno task fmt:check, deno task lint, deno task typecheck, and the focused runtime-support and suite-runner tests.

@codex review

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 371636f4d3